Skip to content

Fix #233: Add missing "common.js" load for "/account/licenses"#234

Closed
smiley wants to merge 1 commit into
SteamDatabase:masterfrom
smiley:patch-1
Closed

Fix #233: Add missing "common.js" load for "/account/licenses"#234
smiley wants to merge 1 commit into
SteamDatabase:masterfrom
smiley:patch-1

Conversation

@smiley

@smiley smiley commented Sep 5, 2025

Copy link
Copy Markdown

scripts/store/account_licenses.js uses GetOption(...) in scripts/common.js but doesn't reference it in its script embed section. While common.js is injected generally for all pages under https://store.steampowered.com/*, looks like it's not there by the time GetOption(...) is called. This settles the race condition. (Tested locally in Developer Mode on Microsoft Edge 139.0.3405.125)

…ses"

`scripts/store/account_licenses.js` uses `GetOption(...)` in `scripts/common.js` but doesn't reference it in its script embed section. While `common.js` is injected generally for all pages under `https://store.steampowered.com/*`, looks like it's not there by the time `GetOption(...)` is called. This settles the race condition.
@xPaw

xPaw commented Sep 5, 2025

Copy link
Copy Markdown
Member

It is referenced by an earlier declaration. This seems like a chrome bug where scripts are injected in wrong order sometimes (I've tried to fix it by adding a timeout). Does restarting the browser fix it for you?

Like I think this is a Chromium bug that they load in the wrong order, but only sometimes.

for example mozilla documents that: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/content_scripts#load_order

@smiley

smiley commented Sep 7, 2025

Copy link
Copy Markdown
Author

Sorry for the delay, looks like the change in c7ac4d6 also works around it. And no, restarting the browser, force updating or force reloading the extension didn't fix it, however now it's magically "fixed" after fiddling with the unpacked extension and going back to the store extension...

@smiley

smiley commented Sep 7, 2025

Copy link
Copy Markdown
Author

Actually, now that I think about it -- if the entire script is in setTimeout(0) now, wouldn't it be easier to change its injection time to document_end or document_idle instead? You also wouldn't need the DOM ready checks then.

@xPaw

xPaw commented Sep 7, 2025

Copy link
Copy Markdown
Member

I would hope even with timeout 0 it would run before document end

@smiley

smiley commented Sep 7, 2025

Copy link
Copy Markdown
Author

But if all of the functionality runs at DOMContentLoaded anyway, wouldn't it be the same as document_end?

@xPaw

xPaw commented Sep 7, 2025

Copy link
Copy Markdown
Member

The injected script runs earlier though.

@smiley

smiley commented Sep 7, 2025

Copy link
Copy Markdown
Author

Oh, right, that solution is fine then

@smiley smiley closed this Sep 7, 2025
@xPaw

xPaw commented Sep 7, 2025

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants